progress: replace setitimer() with alarm() #1960
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carenas
force-pushed
the
noitimer
branch
3 times, most recently
from
526d522 to
e70199f
Compare
setitimer() was marked obsolescent by IEEE Std 1003.1-2017 and is no longer available in the latest edition. While other alternatives of high fidelity are available, its use in progress only requires whole second intervals, so it can be cleanly replaced by a simpler alarm(); do that. MinGW provided its own version of setitimer() so add an equivalent implementation for alarm(), but do the minimum changes required in the timer thread to adapt to the new interface. Remove the compatibility layer for setitimer() and its related struct that are no longer needed, and adjust the build system. Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]>
In a previous commit, sigitimer() was replaced by alarm(), but to keep the timer active, an extra call to `alarm(1)` was added to the signal handler, opening a potential race condition whem the timer is being cleared. To avoid that, add an extra state to set during shutdown and adjust the logic to flag the potential need to update progress into the first bit instead. Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
Error: 0db98c3 was already submitted |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Am 23.08.25 um 15:22 schrieb Carlo Marcelo Arenas Belón via GitGitGadget:
> The first patch does the minimum changes required to swap the underlying
> function, but introduce a race condition that is addressed in the second
> patch.
>
> A third patch that does further changes to the Windows compatibility layer
> was punted.
>
> Carlo Marcelo Arenas Belón (2): progress: replace setitimer() with alarm()
> progress: add a shutting down state to the SIGALRM handler
>
After having looked at the progress code for a bit, see this:
We use SIGALRM to raise a flag that tells the progress code to act in
some way. The progress code does not act asynchronously, but only when
there is an opportunity to look at the flag, i.e., it acts synchronously
in response to a third party (SIGALRM) that told it that it's time to
act.
But we can change the progress code to do the time keeping itself.
Instead of looking whether a flag was raised, we can let it look at the
wall clock and check whether an interval has elapsed.
A prototype implementation looks like the patch below. It works quite
well for `git clone` on Linux. I have even lowered the interval to 1/8th
of a second to get more frequent updates. After a change like this, we
can remove all the creepy SIGALRM/alarm/setitimer stuff.
What do you think?
progress.c | 65 +++++++++++-----------------------------
progress.h | 2 +-
t/helper/test-progress.c | 2 +-
3 files changed, 20 insertions(+), 49 deletions(-)
diff --git a/progress.c b/progress.c
index 8d5ae70f3a..eb1d2f14e0 100644
--- a/progress.c
+++ b/progress.c
@@ -38,6 +38,7 @@ struct throughput {
struct progress {
struct repository *repo;
const char *title;
+ uint64_t last_update;
uint64_t last_value;
uint64_t total;
unsigned last_percent;
@@ -50,57 +51,26 @@ struct progress {
int split;
};
-static volatile sig_atomic_t progress_update;
-
/*
* These are only intended for testing the progress output, i.e. exclusively
* for 'test-tool progress'.
*/
int progress_testing;
uint64_t progress_test_ns = 0;
-void progress_test_force_update(void)
+void progress_test_force_update(struct progress *progress)
{
- progress_update = 1;
+ progress->last_update = 0;
}
-static void progress_interval(int signum UNUSED)
+static bool check_update_delay_expired(struct progress *progress)
{
- progress_update = 1;
-}
-
-static void set_progress_signal(void)
-{
- struct sigaction sa;
- struct itimerval v;
-
- if (progress_testing)
- return;
-
- progress_update = 0;
-
- memset(&sa, 0, sizeof(sa));
- sa.sa_handler = progress_interval;
- sigemptyset(&sa.sa_mask);
- sa.sa_flags = SA_RESTART;
- sigaction(SIGALRM, &sa, NULL);
-
- v.it_interval.tv_sec = 1;
- v.it_interval.tv_usec = 0;
- v.it_value = v.it_interval;
- setitimer(ITIMER_REAL, &v, NULL);
-}
-
-static void clear_progress_signal(void)
-{
- struct itimerval v = {{0,},};
-
- if (progress_testing)
- return;
-
- setitimer(ITIMER_REAL, &v, NULL);
- signal(SIGALRM, SIG_IGN);
- progress_update = 0;
+ uint64_t now = getnanotime();
+ /* about every 1/8th of a second */
+ bool update = (now - progress->last_update) / (1024 * 1024 * 1024 / 8) > 0;
+ if (update)
+ progress->last_update = now;
+ return update;
}
static int is_foreground_fd(int fd)
@@ -109,7 +79,8 @@ static int is_foreground_fd(int fd)
return tpgrp < 0 || tpgrp == getpgid(0);
}
-static void display(struct progress *progress, uint64_t n, const char *done)
+static void display(struct progress *progress, uint64_t n, const char *done,
+ bool progress_update)
{
const char *tp;
struct strbuf *counters_sb = &progress->counters_sb;
@@ -243,15 +214,16 @@ void display_throughput(struct progress *progress, uint64_t total)
tp->last_misecs[tp->idx] = misecs;
tp->idx = (tp->idx + 1) % TP_IDX_MAX;
+ bool progress_update = check_update_delay_expired(progress);
throughput_string(&tp->display, total, rate);
if (progress->last_value != -1 && progress_update)
- display(progress, progress->last_value, NULL);
+ display(progress, progress->last_value, NULL, progress_update);
}
void display_progress(struct progress *progress, uint64_t n)
{
if (progress)
- display(progress, n, NULL);
+ display(progress, n, NULL, check_update_delay_expired(progress));
}
static struct progress *start_progress_delay(struct repository *r,
@@ -262,6 +234,7 @@ static struct progress *start_progress_delay(struct repository *r,
progress->repo = r;
progress->title = title;
progress->total = total;
+ progress->last_update = getnanotime();
progress->last_value = -1;
progress->last_percent = -1;
progress->delay = delay;
@@ -271,7 +244,6 @@ static struct progress *start_progress_delay(struct repository *r,
strbuf_init(&progress->counters_sb, 0);
progress->title_len = utf8_strwidth(title);
progress->split = 0;
- set_progress_signal();
trace2_region_enter("progress", title, r);
return progress;
}
@@ -339,9 +311,9 @@ static void force_last_update(struct progress *progress, const char *msg)
rate = tp->curr_total / (misecs ? misecs : 1);
throughput_string(&tp->display, tp->curr_total, rate);
}
- progress_update = 1;
+ progress->last_update = 0;
buf = xstrfmt(", %s.\n", msg);
- display(progress, progress->last_value, buf);
+ display(progress, progress->last_value, buf, check_update_delay_expired(progress));
free(buf);
}
@@ -374,7 +346,6 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
force_last_update(progress, msg);
log_trace2(progress);
- clear_progress_signal();
strbuf_release(&progress->counters_sb);
if (progress->throughput)
strbuf_release(&progress->throughput->display);
diff --git a/progress.h b/progress.h
index ed068c7bab..cca479bd26 100644
--- a/progress.h
+++ b/progress.h
@@ -9,7 +9,7 @@ struct repository;
extern int progress_testing;
extern uint64_t progress_test_ns;
-void progress_test_force_update(void);
+void progress_test_force_update(struct progress *progress);
#endif
diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c
index 1f75b7bd19..e2ca5fb726 100644
--- a/t/helper/test-progress.c
+++ b/t/helper/test-progress.c
@@ -87,7 +87,7 @@ int cmd__progress(int argc, const char **argv)
progress_test_ns = test_ms * 1000 * 1000;
display_throughput(progress, byte_count);
} else if (!strcmp(line.buf, "update")) {
- progress_test_force_update();
+ progress_test_force_update(progress);
} else if (!strcmp(line.buf, "stop")) {
stop_progress(&progress);
} else {
--
2.51.0.205.g9a02ae2892
|
|
On the Git mailing list, Carlo Marcelo Arenas Belón wrote (reply to this): On Sat, Aug 23, 2025 at 06:24:57PM -0800, Johannes Sixt wrote:
> Am 23.08.25 um 15:22 schrieb Carlo Marcelo Arenas Belón via GitGitGadget:
> > The first patch does the minimum changes required to swap the underlying
> > function, but introduce a race condition that is addressed in the second
> > patch.
> >
> > A third patch that does further changes to the Windows compatibility layer
> > was punted.
> >
> > Carlo Marcelo Arenas Belón (2): progress: replace setitimer() with alarm()
> > progress: add a shutting down state to the SIGALRM handler
> >
> After having looked at the progress code for a bit, see this:
>
> We use SIGALRM to raise a flag that tells the progress code to act in
> some way. The progress code does not act asynchronously, but only when
> there is an opportunity to look at the flag, i.e., it acts synchronously
> in response to a third party (SIGALRM) that told it that it's time to
> act.
>
> But we can change the progress code to do the time keeping itself.
> Instead of looking whether a flag was raised, we can let it look at the
> wall clock and check whether an interval has elapsed.
This is an even better approach indeed, and will lead to an even nicer
cleanup in the Windows emulation code.
> A prototype implementation looks like the patch below. It works quite
> well for `git clone` on Linux. I have even lowered the interval to 1/8th
> of a second to get more frequent updates. After a change like this, we
> can remove all the creepy SIGALRM/alarm/setitimer stuff.
Would you mind cleaning it up and making it a patch I could rebase on?, or
would you rather finish it off, since you also know the Windows parts better?
Carlo |
|
User |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Am 23.08.25 um 21:38 schrieb Carlo Marcelo Arenas Belón:
> Would you mind cleaning it up and making it a patch I could rebase on?, or
> would you rather finish it off, since you also know the Windows parts better?
You can pull a much more polished version from
https://github.com/j6t/git.git progress-wall-clock
I'll wait until CI shows all green before I submit the patches.
-- Hannes
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Sixt <[email protected]> writes:
> We use SIGALRM to raise a flag that tells the progress code to act in
> some way. The progress code does not act asynchronously, but only when
> there is an opportunity to look at the flag, i.e., it acts synchronously
> in response to a third party (SIGALRM) that told it that it's time to
> act.
>
> But we can change the progress code to do the time keeping itself.
> Instead of looking whether a flag was raised, we can let it look at the
> wall clock and check whether an interval has elapsed.
Yes, the use of itimer to only change the flag without doing
anything funky has been a very safe way to use signals, doing only
absolutely minimal thing in the signal handler. Having to rearm the
signal in the signal handler in Carlo's patch made me feel dirtier.
But looking at the wallclock once every iteration of a busy loop?
Operating system folks may have worked hard to minimize the cost of
system calls to gettimeofday() in order to help applications that do
so, but I somehow feel even dirtier to hear proposal to do so to
replace a signal that we set and forget, to be reminded once every
second.
I dunno. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Junio C Hamano <[email protected]> writes:
> Operating system folks may have worked hard to minimize the cost of
> system calls to gettimeofday() in order to help applications that do
> so, but I somehow feel even dirtier to hear proposal to do so to
> replace a signal that we set and forget, to be reminded once every
> second.
I actually think this is probably fine. It is not like we are
spinning only to wait. Every iteration we are doing useful work,
and modern gettimeofday() implementations would be fine with this
use case. |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Am 23.08.25 um 23:33 schrieb Junio C Hamano:
> Yes, the use of itimer to only change the flag without doing
> anything funky has been a very safe way to use signals, doing only
> absolutely minimal thing in the signal handler. Having to rearm the
> signal in the signal handler in Carlo's patch made me feel dirtier.
While this is clean on POSIX, it isn't the only platform where Git runs.
On Windows, this part of the progress indication is as dirty as it can
get. Getting rid of it is a big bonus in my book.
> But looking at the wallclock once every iteration of a busy loop?
>
> Operating system folks may have worked hard to minimize the cost of
> system calls to gettimeofday() in order to help applications that do
> so, but I somehow feel even dirtier to hear proposal to do so to
> replace a signal that we set and forget, to be reminded once every
> second.
I think that ship has sailed already. Look at display_throughput(). One
of the first things it does is to look at the wallclock a.k.a.
getnanotime().
That said, I am not very happy about the new calls introduced in
display_progress(), either. I'll see whether I can produce some
performance measurements.
I observe a behavior change with delayed progress indicators that I have
to understand and fix it before I can submit the cleaned up patches.
-- Hannes
|
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Am 24.08.25 um 00:03 schrieb Johannes Sixt:
> That said, I am not very happy about the new calls introduced in
> display_progress(), either. I'll see whether I can produce some
> performance measurements.
I haven't made up my mind, yet, whether I want to persue the direction
any further. Since we do not have a low-latency implementation of
getnanotime() for all supported systems, calling the function tens of
thousands of times per second could be too burdensome for some of them.
> I observe a behavior change with delayed progress indicators that I have
> to understand and fix it before I can submit the cleaned up patches.
But I found a bug in the delayed progress indicators: the initial delay
time is always just one second instead of the configured time (two
seconds by default).
Below is a fix. This is a minimal patch. It could be extended to reduce
the number of local flag variables in display() and perhaps also reduce
indentation levels with some early returns.
If Carlo wants to advance the original patches, this patch also provides
an obvious point where the alarm clock can be re-armed outside the
signal handler. It would be where we set progress_update = 0.
----- 8< -----
Subject: [PATCH] progress: pay attention to (customized) delay time
Using one of the start_delayed_*() functions, clients of the progress
API can request that a progress meter is only shown after some time.
To do that, the implementation intends to count down the number of
seconds stored in struct progress by observing flag progress_update,
which the timer interrupt handler sets when a second has elapsed. This
works during the first second of the delay. But the code forgets to
reset the flag to zero, so that subsequent calls of display_progress()
think that another second has elapsed and decrease the count again
until zero is reached. Due to the frequency of the calls, this happens
without an observable delay in practice, so that the effective delay is
always just one second.
This bug has been with us since the inception of the feature. Despite
having been touched on various occasions, such as 8aade107dd84
(progress: simplify "delayed" progress API), 9c5951cacf5c (progress:
drop delay-threshold code), and 44a4693bfcec (progress: create
GIT_PROGRESS_DELAY), the short delay went unnoticed.
Copy the flag state into a local variable and reset the global flag
right away so that we can detect the next clock tick correctly.
Since we have not had any complaints that the delay of one second is
too short nor that GIT_PROGRESS_DELAY is ignored, people seem to be
comfortable with the status quo. Therefore, set the default to 1 to
keep the current behavior.
Signed-off-by: Johannes Sixt <[email protected]>
---
Documentation/git.adoc | 2 +-
progress.c | 12 +++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 743b7b00e4..03e9e69d25 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -684,7 +684,7 @@ other
`GIT_PROGRESS_DELAY`::
A number controlling how many seconds to delay before showing
- optional progress indicators. Defaults to 2.
+ optional progress indicators. Defaults to 1.
`GIT_EDITOR`::
This environment variable overrides `$EDITOR` and `$VISUAL`.
diff --git a/progress.c b/progress.c
index 8d5ae70f3a..39a1101420 100644
--- a/progress.c
+++ b/progress.c
@@ -114,16 +114,19 @@ static void display(struct progress *progress, uint64_t n, const char *done)
const char *tp;
struct strbuf *counters_sb = &progress->counters_sb;
int show_update = 0;
+ sig_atomic_t update = progress_update;
int last_count_len = counters_sb->len;
- if (progress->delay && (!progress_update || --progress->delay))
+ progress_update = 0;
+
+ if (progress->delay && (!update || --progress->delay))
return;
progress->last_value = n;
tp = (progress->throughput) ? progress->throughput->display.buf : "";
if (progress->total) {
unsigned percent = n * 100 / progress->total;
- if (percent != progress->last_percent || progress_update) {
+ if (percent != progress->last_percent || update) {
progress->last_percent = percent;
strbuf_reset(counters_sb);
@@ -133,7 +136,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
tp);
show_update = 1;
}
- } else if (progress_update) {
+ } else if (update) {
strbuf_reset(counters_sb);
strbuf_addf(counters_sb, "%"PRIuMAX"%s", (uintmax_t)n, tp);
show_update = 1;
@@ -166,7 +169,6 @@ static void display(struct progress *progress, uint64_t n, const char *done)
}
fflush(stderr);
}
- progress_update = 0;
}
}
@@ -281,7 +283,7 @@ static int get_default_delay(void)
static int delay_in_secs = -1;
if (delay_in_secs < 0)
- delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 2);
+ delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 1);
return delay_in_secs;
}
--
2.51.0.205.g9a02ae2892
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Sixt <[email protected]> writes:
>> Operating system folks may have worked hard to minimize the cost of
>> system calls to gettimeofday() in order to help applications that do
>> so, but I somehow feel even dirtier to hear proposal to do so to
>> replace a signal that we set and forget, to be reminded once every
>> second.
>
> I think that ship has sailed already. Look at display_throughput(). One
> of the first things it does is to look at the wallclock a.k.a.
> getnanotime().
It can be fixed if we wanted to, though, no? Instead of doing all
the computation for the latest lap, and then decide not to show by
looking at the progress_update flag (set by the interrupt), we can
accumulate the total in the progress->throughput struct until we see
the progress_update flag, at which time we can look at the wallclock
time, compute the time difference, perform clever division, etc.
> That said, I am not very happy about the new calls introduced in
> display_progress(), either. I'll see whether I can produce some
> performance measurements.
>
> I observe a behavior change with delayed progress indicators that I have
> to understand and fix it before I can submit the cleaned up patches.
Thanks. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Sixt <[email protected]> writes:
> Subject: [PATCH] progress: pay attention to (customized) delay time
> ...
> until zero is reached. Due to the frequency of the calls, this happens
> without an observable delay in practice, so that the effective delay is
> always just one second.
> ...
> Since we have not had any complaints that the delay of one second is
> too short nor that GIT_PROGRESS_DELAY is ignored, people seem to be
> comfortable with the status quo. Therefore, set the default to 1 to
> keep the current behavior.
OK. This is documenting the established behaviour, which makes
sense.
> struct strbuf *counters_sb = &progress->counters_sb;
> int show_update = 0;
> + sig_atomic_t update = progress_update;
It is somewhat misleading to use sig_atomic_t for "update", which is
never updated via the signal handler. It confused me a bit during
my initial reading. If it were
int update = !!progress_update;
it would have made it more obvious what is going on, at least to me.
In any case, I think it is an excellent idea to clear the global one
first ...
> int last_count_len = counters_sb->len;
>
> - if (progress->delay && (!progress_update || --progress->delay))
> + progress_update = 0;
..., while remembering the fact that progress_update was originally
set or unset, and consistently use the latter in the remainder of
the function, like ...
> + if (progress->delay && (!update || --progress->delay))
> return;
... this one and everywhere below (omitted from quote).
Thanks.
|
|
On the Git mailing list, Carlo Marcelo Arenas Belón wrote (reply to this): On Mon, Aug 25, 2025 at 10:00:25AM -0800, Junio C Hamano wrote:
>
> > struct strbuf *counters_sb = &progress->counters_sb;
> > int show_update = 0;
> > + sig_atomic_t update = progress_update;
>
> It is somewhat misleading to use sig_atomic_t for "update", which is
> never updated via the signal handler. It confused me a bit during
> my initial reading. If it were
>
> int update = !!progress_update;
>
> it would have made it more obvious what is going on, at least to me.
In that case, I would suggest doing instead:
bool update = !!progress_update;
Carlo |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Carlo Marcelo Arenas Belón <[email protected]> writes:
> On Mon, Aug 25, 2025 at 10:00:25AM -0800, Junio C Hamano wrote:
>>
>> > struct strbuf *counters_sb = &progress->counters_sb;
>> > int show_update = 0;
>> > + sig_atomic_t update = progress_update;
>>
>> It is somewhat misleading to use sig_atomic_t for "update", which is
>> never updated via the signal handler. It confused me a bit during
>> my initial reading. If it were
>>
>> int update = !!progress_update;
>>
>> it would have made it more obvious what is going on, at least to me.
>
> In that case, I would suggest doing instead:
>
> bool update = !!progress_update;
Any conventional type we would use for "is it set or not?" that is
not sig_atomic_t is good enough in this context.
The fact that we started adopting "bool" in new code is orthogonal
and a bit off the point. |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Using one of the start_delayed_*() functions, clients of the progress
API can request that a progress meter is only shown after some time.
To do that, the implementation intends to count down the number of
seconds stored in struct progress by observing flag progress_update,
which the timer interrupt handler sets when a second has elapsed. This
works during the first second of the delay. But the code forgets to
reset the flag to zero, so that subsequent calls of display_progress()
think that another second has elapsed and decrease the count again
until zero is reached. Due to the frequency of the calls, this happens
without an observable delay in practice, so that the effective delay is
always just one second.
This bug has been with us since the inception of the feature. Despite
having been touched on various occasions, such as 8aade107dd84
(progress: simplify "delayed" progress API), 9c5951cacf5c (progress:
drop delay-threshold code), and 44a4693bfcec (progress: create
GIT_PROGRESS_DELAY), the short delay went unnoticed.
Copy the flag state into a local variable and reset the global flag
right away so that we can detect the next clock tick correctly.
Since we have not had any complaints that the delay of one second is
too short nor that GIT_PROGRESS_DELAY is ignored, people seem to be
comfortable with the status quo. Therefore, set the default to 1 to
keep the current behavior.
Signed-off-by: Johannes Sixt <[email protected]>
---
Compared to the first round, this replaces sig_atomic_t by int. I
didn't use bool just in case the patch goes on top of a maintenance
track that does not have the "bool is allowed" policy.
Documentation/git.adoc | 2 +-
progress.c | 12 +++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 743b7b00e4..03e9e69d25 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -684,7 +684,7 @@ other
`GIT_PROGRESS_DELAY`::
A number controlling how many seconds to delay before showing
- optional progress indicators. Defaults to 2.
+ optional progress indicators. Defaults to 1.
`GIT_EDITOR`::
This environment variable overrides `$EDITOR` and `$VISUAL`.
diff --git a/progress.c b/progress.c
index 8d5ae70f3a..8315bdc3d4 100644
--- a/progress.c
+++ b/progress.c
@@ -114,16 +114,19 @@ static void display(struct progress *progress, uint64_t n, const char *done)
const char *tp;
struct strbuf *counters_sb = &progress->counters_sb;
int show_update = 0;
+ int update = !!progress_update;
int last_count_len = counters_sb->len;
- if (progress->delay && (!progress_update || --progress->delay))
+ progress_update = 0;
+
+ if (progress->delay && (!update || --progress->delay))
return;
progress->last_value = n;
tp = (progress->throughput) ? progress->throughput->display.buf : "";
if (progress->total) {
unsigned percent = n * 100 / progress->total;
- if (percent != progress->last_percent || progress_update) {
+ if (percent != progress->last_percent || update) {
progress->last_percent = percent;
strbuf_reset(counters_sb);
@@ -133,7 +136,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
tp);
show_update = 1;
}
- } else if (progress_update) {
+ } else if (update) {
strbuf_reset(counters_sb);
strbuf_addf(counters_sb, "%"PRIuMAX"%s", (uintmax_t)n, tp);
show_update = 1;
@@ -166,7 +169,6 @@ static void display(struct progress *progress, uint64_t n, const char *done)
}
fflush(stderr);
}
- progress_update = 0;
}
}
@@ -281,7 +283,7 @@ static int get_default_delay(void)
static int delay_in_secs = -1;
if (delay_in_secs < 0)
- delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 2);
+ delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 1);
return delay_in_secs;
}
--
2.51.0.205.g9a02ae2892
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Sixt <[email protected]> writes:
> Compared to the first round, this replaces sig_atomic_t by int. I
> didn't use bool just in case the patch goes on top of a maintenance
> track that does not have the "bool is allowed" policy.
That's fair. I happened to have chosen v2.51.0 as the base for v1
during today's integration, but as a fix-up topic, you are correct
to point out that I should apply this on top of maint-2.50 or even
older.
Thanks, will requeue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first patch does the minimum changes required to swap the underlying function,
but introduce a race condition that is addressed in the second patch.
A third patch that does further changes to the Windows compatibility layer was punted.
Carlo Marcelo Arenas Belón (2):
progress: replace setitimer() with alarm()
progress: add a shutting down state to the SIGALRM handler
Makefile | 12 ------------
compat/mingw-posix.h | 9 +--------
compat/mingw.c | 46 ++++++++++++++++----------------------------
compat/posix.h | 17 ----------------
configure.ac | 13 -------------
meson.build | 16 ---------------
progress.c | 29 +++++++++++++++-------------
7 files changed, 34 insertions(+), 108 deletions(-)
CC: Nicolas Pitre [email protected]
CC: Johannes Sixt [email protected]
cc: Carlo Marcelo Arenas Belón [email protected]